-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a paragraph about self #8928
Conversation
Co-authored-by: A Thousand Ships <[email protected]>
Co-authored-by: A Thousand Ships <[email protected]>
Thanks for the PR! This is greatly appreciated as it is one of the oldest undocumented features of GDScript. However, in my opinion, in the current edition the emphasis is shifted a little in a wrong direction. This feature is not intended solely for inheritance, as the reader might think. I think In GDScript there is no if has_method("test"):
test() # Error.
self.test() # OK.
call("test") # OK. |
@dalexeev I (hopefully) addressed most of your concerns in e7e4ac8, but i am still not sure how to approach the examples. I have something like this in mind: class_name Item extends Node
# Returns the expected size an item will take up in
# player's inventory (or -1 if it cannot be collected).
func get_size() -> int:
var size = get(&"size")
return size if size else -1
# When player touches an item, collect it if it is
# collectible (i.e. has a `collect` method).
func on_player_touch() -> void:
if has_method(&"collect"):
# collect() # Would be an error!
# self.collect() # @dalexeev This is also appears to be an error
# on latest 4.2, perhaps you misremembered this part?
call(&"collect")
# .. another file ..
class_name Potion extends Item
var size := 2
func collect() -> void:
print("Collected a potion!") But it looks pretty verbose and (in my view) not immediately understandable without additional comments. Do you have any better ideas? |
dead3eb
to
e7e4ac8
Compare
@dalexeev As i haven't heard anything negative towards the proposed example, i went ahead and replaced the weapon/pistol example with this new one. I also added a |
@elenakrittik Could you look into replying to the above suggestions (or applying them directly) so this PR can be merged? Thanks in advance 🙂 |
Much sorry, i'll apply the automatic change right away but i unfortunately will not be able to address dalexeev's feedback until after a week or so. Once i'm back i'll make sure to drive this to the finish line! |
Co-authored-by: RedMser <[email protected]>
Co-authored-by: Danil Alexeev <[email protected]>
Sorry for the long wait! I believe everything should be good now. |
set("not_exist", 123) # No error or warning.
self["not_exist"] = 123 # Runtime error.
self.not_exist = 123 # Runtime error.
not_exist = 123 # Compile time error.
print(get("not_exist")) # No error or warning.
print(self["not_exist"]) # Runtime error.
print(self.not_exist) # Runtime error.
print(not_exist) # Compile time error. |
Co-authored-by: Danil Alexeev <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If dalexeev approves it's good.
I suggested a couple changes to better follow the style guide.
Co-authored-by: tetrapod <[email protected]>
Is there someone "final" i should request a review from, or does the "review queue" move automatically? |
It's pretty informal for the docs repo, we don't have as much of a process as the main repo does. In most cases PRs are good to merge once it gets an approval from a subject matter expert (dalexeev in this case) and a general docs reviewer (me in this case). Sometimes we merge things with less approvals than that, even, if it's a very obvious change. This is good to merge I just didn't get around to it yet. |
Merged! Thank you. |
Adds a paragraph about the
self
keyword and documents that it can be used to refer to variablesdefined in subclasses of current class.
Closes godotengine/godot#87944